Start accounting for notificationPreferences in the unread indicator for Chat Rooms #3766

yuwenmemon · 2021-06-25T23:13:57Z

Details

When users have notificationPreferences in E.Chat for a given, room (comes live with https://github.com/Expensify/Expensify/issues/161785 cc @jasperhuangg) - follow them for push notifications within the app.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/161781

Tests/QA

Set your notification preferences for a policy's announce room to "always" by running the following JS in E.Web (User 1):

p = Policy.getCurrent();
announceID = p.policy.chatReportIDAnnounce
API._performPOSTRequest("Report_UpdateNotificationPreference", {
    reportID: announceID,
    notificationPreference: 'always'
});

For Phones, keep E.Chat running in the background for User 1, for Web/Desktop, have the tab or window open but not focused.
Open up E.Chat in another client from a different account that's also in the same policy (User 2)
From that account send 4 messages in the #announce room for that policy.
Make sure that the app indicator for the client User 1 is in shows 4

Set your notification preferences for a policy's announce room to "daily" by running the following JS in Web Expensify (OldDot) (User 1):

p = Policy.getCurrent();
announceID = p.policy.chatReportIDAnnounce
API._performPOSTRequest("Report_UpdateNotificationPreference", {
    reportID: announceID,
    notificationPreference: 'daily'
});

For Phones, keep E.Chat running in the background for User 1, for Web/Desktop, have the tab or window open but not focused.
Open up E.Chat in another client from a different account that's also in the same policy (User 2)
From that account send a message in the #announce room for that policy.
Make sure that the app indicator for the client User 1 is using shows 1 or is simply blank but present.

Set your notification preferences for a policy's announce room to "mute" by running the following JS in Web Expensify (OldDot) (User 1):

p = Policy.getCurrent();
announceID = p.policy.chatReportIDAnnounce
API._performPOSTRequest("Report_UpdateNotificationPreference", {
    reportID: announceID,
    notificationPreference: 'mute'
});

For Phones, keep E.Chat running in the background for User 1, for Web/Desktop, have the tab or window open but not focused.
Open up E.Chat in another client from a different account that's also in the same policy (User 2)
From that account send a message in the #announce room for that policy.
Make sure that you do not see any indicator update on the app icon for User 1

Tested On

Screenshots

Web

Mobile Web

N/A

Desktop

iOS

Android

Not Applicable

…erences

jasperhuangg · 2021-06-28T03:19:02Z

Hey @yuwenmemon! I can't seem to get this to work on Desktop when testing 'always'. I'm pretty sure I've followed the right steps, but the indicator always shows up as 1 regardless of how many messages I send, am I missing anything? Here's a screen capture:

demo.mp4

yuwenmemon · 2021-06-28T23:18:00Z

@jasperhuangg yep, unfortunately that's expected behavior for Desktop as the Electron API fails otherwise. See my QA step here:

Make sure that the app indicator for the client User 1 is using shows 1 or is simply blank but present.

and this comment here:
https://github.com/Expensify/Expensify.cash/pull/3766/files#diff-9276f078b750aa322d09bda48c79eb8034941df5b0f0f0c4f8b898cf9c652c41R244-R247

…n-LHNPrefs

desktop/main.js

src/libs/UnreadIndicatorUpdater/updateUnread/index.website.js

src/libs/UnreadIndicatorUpdater/index.js

thienlnam

Awesome, thanks for entertaining my questions! Changes look good

yuwenmemon · 2021-06-29T03:33:28Z

Didn't see that you referring to an always preference @jasperhuangg - my bad. Just tested that again, not seeing what you're seeing. I have the always user signed in on the desktop app in this scenario:

jasperhuangg

Looks great and tests well :)

OSBotify · 2021-06-29T04:21:09Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-06-29T19:26:17Z

🚀 Deployed to staging in version: 1.0.74-1🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

isagoico · 2021-06-30T11:51:11Z

@jasperhuangg @yuwenmemon Not sure if this is testable for us at the moment. I tried creating a workspace and adding members but I'm stuck on this page. It won't let me add members or create the chat rooms.

yuwenmemon · 2021-06-30T16:44:25Z

@isagoico can you add those folks to a policy in the "normal" Expensify web app?

isagoico · 2021-06-30T23:43:17Z

I created the workspace on an account and was unable to find the Policy in the classic app to add the members there. Not sure if I'm missing anything.

yuwenmemon · 2021-07-01T00:26:31Z

@isagoico perhaps just to keep things simple can you create the policy and add the members via the classic app?

isagoico · 2021-07-01T11:00:50Z

mmm I tried creating the policy in classic app and adding the members there and the workspace does not appear in e.cash. This is what I've tried, let me know if I'm missing something:

Create workspace in e.cash
Navigated to e.com to find the policy
Policy is not displayed in e.com
Tried accessing the policy via de Policy ID number in e.com (did not work)

Since above didn't work

I created a policy in e.com in a account that didn't have any previous policies
Added some members to the policy
Checked e.cash if I could see the workspace, nothing was displayed

yuwenmemon · 2021-07-01T21:12:17Z

I guess I'm a bit confused as to why you're looking for a workspace. We're not testing workspaces in this PR, we want to test the #announce room.

isagoico · 2021-07-01T21:21:51Z

OH I totally thought that the rooms like #announce were created when a workspace was set up. Not sure how to access the #announce room then 🤔 should it automatically appear on any account?

yuwenmemon · 2021-07-01T21:24:27Z

Try searching for the policy name OR announce in the E.Chat app:

isagoico · 2021-07-02T13:11:06Z

That worked! Thank you so much for the clarification, I was completely of the loop on how to access the rooms.
I tried following the steps and got this errors when running the snippets

Also when sending the messages, no notifications were received on the User 2 device (in my case it was my android phone)

yuwenmemon · 2021-07-02T14:45:13Z

@isagoico You'll need to run that snippet in the classic app.

There's no applicable test for this for Android, so that makes sense.

isagoico · 2021-07-05T13:25:27Z

Tested this and it was a pass:

First scenario:

Got notifications for each message in this case 4

Second scenario:

I got no notification but I did have the NEW! indicator.

Third scenario:

No notification or NEW! indicator was displayed.

yuwenmemon added 2 commits June 24, 2021 17:11

Design Doc Specs of the Sidebar / GSD Mode Aspect of NotificationPref…

9335df5

…erences

Add fix for desktop

45dbe87

yuwenmemon requested a review from jasperhuangg June 25, 2021 23:13

yuwenmemon requested a review from a team as a code owner June 25, 2021 23:13

yuwenmemon self-assigned this Jun 25, 2021

MelvinBot requested review from thienlnam and removed request for a team June 25, 2021 23:14

yuwenmemon changed the title ~~Start accounting for notificationPreferences in the LHN for Chat Rooms~~ Start accounting for notificationPreferences in the unread indicator for Chat Rooms Jun 28, 2021

Merge branch 'main' of github.com:Expensify/ReactNativeChat into yuwe…

1d72d95

…n-LHNPrefs

thienlnam reviewed Jun 28, 2021

View reviewed changes

desktop/main.js Show resolved Hide resolved

src/libs/UnreadIndicatorUpdater/updateUnread/index.website.js Show resolved Hide resolved

src/libs/UnreadIndicatorUpdater/index.js Show resolved Hide resolved

thienlnam previously approved these changes Jun 28, 2021

View reviewed changes

Update if/else error

fd74b95

yuwenmemon dismissed thienlnam’s stale review via fd74b95 June 28, 2021 23:57

jasperhuangg approved these changes Jun 29, 2021

View reviewed changes

jasperhuangg merged commit 1bfa5d2 into main Jun 29, 2021

jasperhuangg deleted the yuwen-LHNPrefs branch June 29, 2021 04:20

This was referenced Jun 29, 2021

Deploy Checklist: Expensify.cash 2021-06-25 #3759

Closed

Deploy Checklist: Expensify.cash 2021-06-29 #3802

Closed

isagoico mentioned this pull request Jul 5, 2021

Create the Report Details Page #3742

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start accounting for notificationPreferences in the unread indicator for Chat Rooms #3766

Start accounting for notificationPreferences in the unread indicator for Chat Rooms #3766

yuwenmemon commented Jun 25, 2021 •

edited by jasperhuangg

Loading

jasperhuangg commented Jun 28, 2021

yuwenmemon commented Jun 28, 2021

thienlnam left a comment

yuwenmemon commented Jun 29, 2021

jasperhuangg left a comment

OSBotify commented Jun 29, 2021

OSBotify commented Jun 29, 2021

isagoico commented Jun 30, 2021

yuwenmemon commented Jun 30, 2021

isagoico commented Jun 30, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 1, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 1, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 2, 2021

yuwenmemon commented Jul 2, 2021

isagoico commented Jul 5, 2021 •

edited

Loading

Start accounting for notificationPreferences in the unread indicator for Chat Rooms #3766

Start accounting for notificationPreferences in the unread indicator for Chat Rooms #3766

Conversation

yuwenmemon commented Jun 25, 2021 • edited by jasperhuangg Loading

Details

Fixed Issues

Tests/QA

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

jasperhuangg commented Jun 28, 2021

yuwenmemon commented Jun 28, 2021

thienlnam left a comment

Choose a reason for hiding this comment

yuwenmemon commented Jun 29, 2021

jasperhuangg left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 29, 2021

OSBotify commented Jun 29, 2021

isagoico commented Jun 30, 2021

yuwenmemon commented Jun 30, 2021

isagoico commented Jun 30, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 1, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 1, 2021

yuwenmemon commented Jul 1, 2021

isagoico commented Jul 2, 2021

yuwenmemon commented Jul 2, 2021

isagoico commented Jul 5, 2021 • edited Loading

yuwenmemon commented Jun 25, 2021 •

edited by jasperhuangg

Loading

isagoico commented Jul 5, 2021 •

edited

Loading